Skip to content

fix(spawn): reject primary checkout by filesystem identity in worktree guard - #60

Merged
withally merged 4 commits into
mainfrom
fm/fm-spawn-identity-guard-i1
Aug 20, 2026
Merged

fix(spawn): reject primary checkout by filesystem identity in worktree guard#60
withally merged 4 commits into
mainfrom
fm/fm-spawn-identity-guard-i1

Conversation

@withally

Copy link
Copy Markdown
Owner

Intent

Fix upstream issue kunchenguid#2654 on the current Firstmate fork only. Make bin/fm-spawn.sh worktree isolation refuse launch whenever the resolved task worktree is the primary checkout by filesystem identity, meaning the same device and inode, rather than textual path equality, including a case-variant spelling on a case-insensitive macOS filesystem. The identity mechanism must work with BSD stat on macOS and GNU stat on Linux. Every same-directory comparison that decides spawn isolation must use identity, including settle-loop primary detection and candidate stability, relaunch endpoint matching, git worktree-root validation, and primary-checkout rejection; identity-read failures must fail loudly instead of degrading to text. Do not weaken the guard, redefine isolation beyond this defect, branch from a fresh-upstream integration base, perform a migration or cutover, switch homes, or expand beyond the identity fix and its tests. Add a portable regression in tests through the executable public interface, never source-byte assertions, using a real same-inode and different-text alias that would fail if reverted to text. Add a live opt-in macOS proof that uses a real case variant on a case-insensitive filesystem and fails loudly naming the platform rather than silently degrading. Capture an exact refusal on this case-insensitive macOS host and prove that the legitimate pooled linked worktree at this task path, which shares the primary checkout Git common directory, is still accepted. Keep the fix and colocated tests shellcheck-clean through bin/fm-lint.sh, follow the one-owner rule, use plain dashes in Markdown, add no agent co-author, and ship one no-mistakes PR against the current fork with CI-ready checks green.

What Changed

  • Add path_filesystem_identity/path_identity_or_refuse helpers to bin/fm-spawn.sh that read a device:inode pair via BSD stat on macOS and GNU stat on Linux, and reroute every same-directory spawn comparison — settle-loop primary detection, candidate stability, relaunch endpoint matching, git worktree-root validation, and primary-checkout rejection — through identity instead of path text, so a case-variant spelling of the primary checkout on a case-insensitive filesystem is now refused; identity-read failures fail loudly and abort rather than degrading to text.
  • Add tests/fm-spawn-worktree-identity-live.test.sh, an opt-in (FM_SPAWN_CASE_IDENTITY_LIVE=1) macOS proof that drives the fm-spawn CLI with a real case-variant primary path and requires the refusal, failing loudly by platform name off Darwin or on a case-sensitive fixture; registered under the live-harness-optin family in bin/fm-test-run.sh.
  • Extend tests/fm-tangle-guard.test.sh with a portable identity-primitive regression (distinct spellings of one directory compare equal, distinct directories differ, missing path fails) that would fail if the comparison reverted to text, plus a CLI-level test that the settle loop refuses the primary checkout itself.

Risk Assessment

✅ Low: The identity primitive is portable and correct, every isolation-deciding comparison now routes through it with loud-fail semantics, tests execute real interfaces, and the only residual (Linux guard-call-site coverage) is an explicitly user-accepted feasibility tradeoff.

Testing

Ran the targeted spawn-identity suite: the opt-in live macOS proof and the portable tangle-guard tests all pass, capturing an exact primary-checkout refusal on this case-insensitive host and confirming a legitimate pooled linked worktree is still accepted. Two revert experiments on a throwaway copy confirmed the tests are genuine regressions — raw-text revert breaks the portable test, and canonicalized-text (pwd -P) revert reproduces the original case-variant tangling bug and is caught by the live proof. Worktree left clean.

Evidence: Live macOS case-identity proof + regression transcript
### Live macOS case-insensitive worktree-identity proof (opt-in) ###
# command: FM_SPAWN_CASE_IDENTITY_LIVE=1 tests/fm-spawn-worktree-identity-live.test.sh
# platform: Darwin
# primary: /var/folders/3n/3wfcplrn3clf44hjjkfgq8t00000gn/T//fm-spawn-worktree-identity-live.cw2s4u/primaryproject
# pane cwd: /var/folders/3n/3wfcplrn3clf44hjjkfgq8t00000gn/T//fm-spawn-worktree-identity-live.cw2s4u/PrimaryProject
# exit: 1
warning: /var/folders/3n/3wfcplrn3clf44hjjkfgq8t00000gn/T//fm-spawn-worktree-identity-live.cw2s4u/home/data/case-identity-live/brief.md records no delivery contract line (scaffolded before ship briefs recorded one); launching on the explicit --mode no-mistakes - confirm its definition of done matches
error: treehouse get did not enter a worktree within 60s; inspect window firstmate:fm-case-identity-live
ok - Darwin case-insensitive filesystem: fm-spawn refuses the primary checkout by identity

### Portable regression + settle-loop refusal + linked-worktree acceptance ###
ok - fm-spawn: aborts unless the resolved worktree is a genuine, isolated worktree
ok - path_filesystem_identity: distinct spellings of one dir identify equal; different dirs differ; missing path fails
ok - fm-spawn: settle loop refuses the primary checkout through the CLI
Evidence: Canonicalized-text revert reproduces the defect (case-variant primary wrongly spawned)
# exit: 0
spawned case-identity-live ... worktree=.../PrimaryProject
not ok - case-variant primary checkout should be refused: expected exit 1, got 0

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info
  • ⚠️ tests/fm-tangle-guard.test.sh:226 - tests/fm-tangle-guard.test.sh:221 (test_spawn_same_identity_alias_abort) does not fail if the fix is reverted to text comparison, so it does not satisfy the intent's required "portable regression ... using a real same-inode and different-text alias that would fail if reverted to text." The fixture's distinct spelling is a leading // (pane_alias="/$proj", line 226). But every identity comparison in fm-spawn.sh is computed from paths already canonicalized by real_path_or_raw (cd … && pwd -P), and pwd -P collapses // to / (verified live: //tmp/x/tmp/x). Trace: fake tmux emits //tmp/.../proj; settle loop sets p_real=real_path_or_raw(p)=/private/tmp/.../proj, which equals PROJ_ABS_REAL. Under the NEW code project_identity==p_identity → refuse; under the OLD text code p_real != PROJ_ABS_REAL is false → also refuse. Both paths yield the same "treehouse get did not enter a worktree" message the test asserts, so the test is a tautology w.r.t. the identity change. Only the live macOS case-variant proof (tests/fm-spawn-worktree-identity-live.test.sh) genuinely exercises text-vs-identity, because macOS pwd -P preserves caller casing while sharing the inode. Recommend replacing the // fixture with one whose distinct spelling survives pwd -P canonicalization yet shares the inode (e.g., a symlink-free case variant guarded to the case-insensitive branch, or feeding the raw pane path before canonicalization), or acknowledge in-test that only the live proof distinguishes identity from text.

🔧 Fix: test(spawn): make portable identity regression fail on text revert
1 warning still open:

  • ⚠️ tests/fm-tangle-guard.test.sh:253 - test_spawn_same_identity_alias_abort only genuinely fails on a text-comparison revert on case-INsensitive filesystems (macOS default). On case-sensitive Linux, no same-inode/different-pwd-P-text pair can be built without root (I confirmed live that a symlink-directory alias collapses under pwd -P, and // is stripped too), so the test takes the fallback branch (lines 252-261) that asserts only the exact-path primary refusal. That fallback passes identically whether the guard uses identity or text, so on Linux it provides zero revert protection. This diverges from the round-1 fix instruction ('genuinely fails on a textual-comparison revert on both Linux and macOS') and from the intent's 'portable regression ... that would fail if reverted to text.' Mitigating facts: (1) the prescribed symlink-directory mechanism is provably infeasible through real_path_or_raw's pwd -P canonicalization, so the fixer's case-variant substitution is a defensible engineering choice; (2) this run's delivery/CI host is Darwin, where the identity branch runs and the regression does genuinely distinguish identity from text, so the core round-1 tautology is resolved on the actual host. Recommend either accepting the documented case-insensitive-only coverage (the comments are honest about the limitation) or, if Linux CI must catch reverts, gating a root/bind-mount-based same-inode fixture behind an opt-in rather than silently degrading. Ask the user which they prefer given the feasibility constraint.

🔧 Fix: test(spawn): portable direct identity test replaces alias e2e
1 info still open:

  • ℹ️ tests/fm-tangle-guard.test.sh:56 - The comment claims test_path_filesystem_identity 'fails if the guard's comparison is reverted to text ... on both Linux and macOS.' Precisely, this test exercises only the path_filesystem_identity primitive, not the guard call sites (validate_spawn_worktree / settle loop). Reverting a call site to text (e.g. [ "$wt_real" != "$wt_top_real" ]) leaves the primitive defined-but-unused, so this portable test would still pass; only the macOS-only opt-in live test catches a guard-level text revert, and it does not run in default CI. This residual Linux coverage gap is the exact feasibility tradeoff the user explicitly approved in round 2 (no same-inode/different-canonical-text pair is constructible on case-sensitive Linux without root, because real_path_or_raw's pwd -P collapses .., symlinks, and //). Behavior is correct and matches authoritative intent; noting for record honesty only.
✅ **Test** - passed

✅ No issues found.

  • FM_SPAWN_CASE_IDENTITY_LIVE=1 bash tests/fm-spawn-worktree-identity-live.test.sh — live macOS case-insensitive refusal proof (exit 1, exact settle-loop refusal captured)
  • bash tests/fm-tangle-guard.test.sh — all guards pass incl. test_path_filesystem_identity, test_spawn_primary_copy_abort, and test_spawn_isolation_abort (linked-worktree acceptance)
  • Revert experiment (throwaway copy): replaced path_filesystem_identity with raw-text return → test_path_filesystem_identity fails ('identity is not a device:inode pair')
  • Revert experiment: replaced path_filesystem_identity with canonicalized pwd -P text → live macOS proof fails, case-variant primary wrongly 'spawned' (exit 0)
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@withally
withally merged commit 35d1bd2 into main Aug 20, 2026
14 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant